🦋 Changeset detected

Latest commit: 9f7d1f6

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Hey! Yeah the linting/formatting parts of my PR caused me a bit of a headache too... I did run the formatting script before committing anything, which I would have thought would solve the problem! I guess not!

I would actually say that the underlying problem may be either my individual setup, or the global setup for linting/formatting in the package as a whole. (From personal experience I know how difficult it is to manage it all properly in a massive monorepo).

Do you think it'd be easier to wait for my PR to be merged - then applying the formatting fixes? (I'd actually be interested to see if you got any different results from running the pretter:check/write scripts in my branch)

Realistically if the issue is stemming from a formatting mismatch from either my machine or the monorepo as a whole - Fixing that may help to resolve the problem?

Obviously i'm happy to pull out the core parts of my PR into this PR if that's truly necessary

@kierancap thanks for you answer.

After looking back at your PR, it seems that the formatting is a smaller problem since your latest commits. There are way fewer space changes. So this is not a problem anymore.

I rebased your work on my PR so here is a change log. So you have the choice to work on this one or yours.

Fixed:

Use of "any"

We were losing a lot of benefit from TypeScript static typing.

This was caused by the partial changes to the peer and dev dependencies. There are a lot of packages to update including React, React-dom, drei, and three-fiber which prevents the need to "any" all the "Tag" related types.

Using null over undefined

Some "useRef" are initialized with "null" while they were initialized with implicit undefined.

before: useRef()
now: useRef(null)

which is different from useRef(undefined) undefined !== null

To fix:

Potential breaking change on the context.

I rebased the work on jest setup and SprintContext so all the tests pass now.

But there is a catch.

Those changes were made:

(Note : I use ISpringContext for the interface to diff from the SpringContext object.)

The current version exports a context object with the following signature :
type SprintContext = React.Context<ISpringContext> & React.FC<PropsWithChildren<ISpringContext>>

But now it splits the exported context in two parts:
type SpringContextProvider = React.Provider<ISpringContext>
and
type SpringContext = React.Context<ISpringContext>

Given that the context provider was exported at the package level, it will break any project that uses SpringContext.

Ex: If my project uses the old version, switching to this new version will break this code:

import { SpringContet } from 'react-spring';

export myComp = function({children, ...props}) {
  return <SpringContext {...props}>
      {children}
    </SpringContext>
}

So, either we bump to a major version due to the breaking change, or we stay at patch/minor level and find a way to keep the same package interface.

@kierancap thanks for you answer.

After looking back at your PR, it seems that the formatting is a smaller problem since your latest commits. There are way fewer space changes. So this is not a problem anymore.

I rebased your work on my PR so here is a change log. So you have the choice to work on this one or yours.

Fixed:

Use of "any"

We were losing a lot of benefit from TypeScript static typing.

This was caused by the partial changes to the peer and dev dependencies. There are a lot of packages to update including React, React-dom, drei, and three-fiber which prevents the need to "any" all the "Tag" related types.

Using null over undefined

Some "useRef" are initialized with "null" while they were initialized with implicit undefined.

before: useRef()

now: useRef(null)

which is different from useRef(undefined) undefined !== null

To fix:

Potential breaking change on the context.

I rebased the work on jest setup and SprintContext so all the tests pass now.

But there is a catch.

Those changes were made:

(Note : I use ISpringContext for the interface to diff from the SpringContext object.)

The current version exports a context object with the following signature :

type SprintContext = React.Context<ISpringContext> & React.FC<PropsWithChildren<ISpringContext>>

But now it splits the exported context in two parts:

type SpringContextProvider = React.Provider<ISpringContext>

and

type SpringContext = React.Context<ISpringContext>

Given that the context provider was exported at the package level, it will break any project that uses SpringContext.

Ex: If my project uses the old version, switching to this new version will break this code:

import { SpringContet } from 'react-spring';



export myComp = function({children, ...props}) {

  return <SpringContext {...props}>

      {children}

    </SpringContext>

}

So, either we bump to a major version due to the breaking change, or we stay at patch/minor level and find a way to keep the same package interface.

Hey!

So...

Any

The any was due to polymorphic components (if i'm remembering right - really annoying). If you were able to tidy those up that's great!

Refs

Completely agreed on the approach to remove undefined from useRef.

Context

I'm nearly certain that with the newest types derived from React, that merging a Provider AND Context initiator into the same object will be nigh on impossible. That's why that old setup was so un-idiomatic and seemed to chip away at the underlying context logic to allow for it to happen.

I would definitely consider a Major bump here - Josh also agreed on the context change in my initial PR.

The Provider paradigm for Context is quite popular these days, and is way simpler on the DX to use. Obviously it does give us a breaking change, but for overall maintainability I think that's a valued change

@kierancap about the breaking change: indeed, I found the message where @joshuaellis agrees.

How do you want to proceed? Either you cherry pick from mine and I close this one or we continue on this one.

It remains:

• document the breaking change
• test with react-native

Assuming the breaking change, the minimum requirements is now React 19

• updated react-native to 0.78
• updated react-konda to 19

added a breaking change section in doc

fix types error in demo due to mutliple react version:

• updated styles-component
• force to use newer version of react-transition-group (dependency of react-select)

@kierancap about the breaking change: indeed, I found the message where @joshuaellis agrees.

How do you want to proceed? Either you cherry pick from mine and I close this one or we continue on this one.

It remains:

• document the breaking change

• test with react-native

I personally feel like you're making really good progress on this - So if you want to pick up where I left off (full transparency, I won't have much time to keep up with this for the next few weeks). I'm happy for you to continue your work on this one.

I can alert on my PR that we move to this one and I can close my PR.

Would that suit you? If you're happy to adopt my context change (thats the only real major change to the upgrade from my PR - ignoring the test fixes and the useRef init values)

Thank you mate!

(The alternative is that we keep my branch active and you can submit PRs to that branch directly if you wish)

@kierancap ok, I can continue from here. I will check your last commit but I think I am up to date.

Crash on my real life tests on React Native:

import { animated } from '@react-spring/native';

• passes with React Native 0.76
• breaks with React Native > 0.78

Cause:

The package @react-spring/native is not resolved with the right path by Metro. Indeed, the ESM package is used which is not compatible with RN (RN uses only CommonJS).

It is due to the Metro breaking change since v0.82, about the "exports" support in package.json.

Metro now uses the exports declaration prior to other entry points. So:

• on Metro < 0.82 the resolver uses the "main" entry point in the package.json which resolves to the CJS module
• on Metro >= 0.82 the resolver uses the "exports" entry point in the package.json which resolves to the ESM module

Impact:

• All RN projects using >= v0.78 will break.

Proposal

To handle that we need to find two entry points in package.json.

EDIT: this proposal has been successfully tested in a client project.

@joshuaellis I have a proposal on this: as far as I know, Metro only uses CommonJS as module system, so:

• we remove the compilation in ES module in react-spring/native
• we set the following in targets/native/package.json

"main": "./dist/cjs/index.js",
"types": "./dist/cjs/react-spring_native.development.d.ts",
"exports": {
    "./package.json": "./package.json",
    ".": {
      "types": "./dist/cjs/react-spring_native.development.d.ts",
      "default": "./dist/cjs/index.js"
    }
  },

gentle ping @joshuaellis